-
Notifications
You must be signed in to change notification settings - Fork 3.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Revert ExecutionContext.global to not be a BatchingExecutor #9270
Conversation
Oh *?^!@#?#Q@ I didn't intend to merge this, I was on the wrong tab... This needs a follow-up at least in documentation, to tell people about the batching alternative and how to get it
Here is how users can create a batching EC instance in their codebase (code adapted from a draft by @viktorklang). Maybe this can be improved further, feedback welcome. 2.13.x...lrytz:custom-batching-ec We also plan to ship the batching EC in scala-library-next. If preferred, we can revert this PR and re-submit it. |
@lrytz I have multiple issues with the proposal:
|
My assumption is that the actual EC used in an application is always imported (or defined) by the application's own source code - is that wrong? Are there libraries out there that use an EC internally without requiring it to be provided as parameter? To me this would seem to be a problematic design; any application that doesn't use
Yes, we're making the
This looks reasonable to me, the risk of it causing problems in 2.13.x maintenance seems really low.
I agree this is a concern, but in my opinion the benefits of reverting |
What people are recommended to do, and what they actually do, was what ended us up in this situation in the first place :)
BatchingExecutor might not be an issue, but leaking internal Promise implementation details is a bit of a no-starter since it completely prevents the implementation to evolve. Also consider ScalaJS.
I hope you're right! |
Two more things we can do:
|
@lrytz Adding the |
This could be done with #8820 |
That'd be welcome, thank you! |
NOTE This is a change in behaviour. If you make use of
scala.concurrent.ExecutionContext.global
you maywant to adapt your code. Please note that Akka uses its own execution contexts (dispatchers) so is
unaffected.
In 2.13.0 we changed the behaviour of
ExecutionContext.global
so that it was more "opportunistic", byattempting to batch nested task and execute them on the same thread as the enclosing task. Unfortunately this
relies heavily on the correct usage of the
blocking { }
wrapper on any long-running and/or blocking tasks;code that didn't could end up executing serially instead of in parallel. As we believe that correct usage of
blocking
isn't as well followed as hoped we decided to revertExecutionContext.global
to its 2.12behaviour and introduce
ExecutionContext.opportunistic
for users that want to switch to it.In order to maintain forwards and backwards binary compatibility of the scala standard library, the usage
instructions require some indirection. Library authors can use the opportunistic execution context for all
Scala 2.13.x versions like so:
While application authors just need to gain access to the
private[scala]
field, which they can do by:object
in thescala
package (example below).private[scala]
is emitted aspublic
in Java bytecode.For example (option 1):
Links:
Fixes scala/bug#12089